Skip to content

Conversation

@Chao1Han
Copy link
Contributor

@Chao1Han Chao1Han commented Sep 22, 2025

For split initXCCLComm and getXCCLComm. align with cuda.

Copilot AI review requested due to automatic review settings September 22, 2025 05:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR separates communication initialization logic from the communication retrieval process by splitting the getXCCLComm method into two distinct functions.

  • Extracted communication lookup logic into a new getXCCLComm method that only retrieves existing communicators
  • Created a new initXCCLComm method that handles the initialization of new communicators
  • Updated call sites to first check for existing communicators before initializing new ones

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/xccl/ProcessGroupXCCL.hpp Added method declaration for new getXCCLComm overload and initXCCLComm method
src/xccl/ProcessGroupXCCL.cpp Implemented the separation of concerns by creating distinct lookup and initialization methods, updated call sites

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@dvrogozh dvrogozh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chuanqi129 chuanqi129 added this pull request to the merge queue Oct 27, 2025
Merged via the queue into main with commit f6eaac8 Oct 27, 2025
25 checks passed
@chuanqi129 chuanqi129 deleted the xccl/comm branch October 27, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants